Conversation
| /// | ||
| /// Note that this does *not* match the PostGIS implementation. | ||
| /// In particular, the GEOS/PostGIS "pctconvex" parameter, which extends | ||
| /// from 0..1 does not match this kernel's "concavity" (0..Infinity). |
There was a problem hiding this comment.
The Concave Hull algorithm is completely new, but its old output didn't match either. This kernel isn't enabled by default for this reason, but I added this note because in reading the docs it seems that the parameter doesn't carry the same meaning. If we did expose this we might have to give it another name.
| // #[test] | ||
| // fn test_random_linestring_to_linestring_distance() { | ||
| // // Test linestring-to-linestring distance with random inputs | ||
| // for i in 0..100 { | ||
| // let seed1 = 77777 + i * 59; | ||
| // let seed2 = 88888 + i * 61; | ||
|
|
||
| // let ls1 = generate_random_linestring(seed1, 3 + (i % 3) as usize); // 3-5 points | ||
| // let ls2 = generate_random_linestring(seed2, 3 + ((i + 1) % 3) as usize); // 3-5 points | ||
|
|
||
| // let concrete_dist = Euclidean.distance(&ls1, &ls2); | ||
| // // Use our actual generic implementation via nearest_neighbour_distance | ||
| // let generic_dist = nearest_neighbour_distance(&ls1, &ls2); | ||
|
|
||
| // assert_relative_eq!( | ||
| // concrete_dist, | ||
| // generic_dist, | ||
| // epsilon = 1e-10, | ||
| // max_relative = 1e-10 | ||
| // ); | ||
| // } | ||
| // } |
There was a problem hiding this comment.
@Kontinuation Can you look into these two tests? I am wondering if the mismatch is a bug on our side or a bug on their side (or unrelated and I'm not understanding the test).
The failure is:
thread 'algorithm::line_measures::metric_spaces::euclidean::utils::tests::test_random_linestring_to_linestring_distance' panicked at rust/sedona-geo-generic-alg/src/algorithm/line_measures/metric_spaces/euclidean/utils.rs:1588:13:
assert_relative_eq!(concrete_dist, generic_dist, epsilon = 1e-10, max_relative = 1e-10)
left = 28.34636916784935
right = 21.713575661323034
(i.e., the algorithm here is giving smaller distances than whatever is producing concrete_dist)
There was a problem hiding this comment.
Preliminary investigations indicate that our calculations are correct, and geo 0.32.0 is incorrect.
I'll look into it further and see if I can submit a patch to georust/geo.
2 participants
Updates the geo crate to 0.32.0
Closes #455
Closes #491
Replaces #549